Deprecate runtime telemetry individual metric classes#16064
Deprecate runtime telemetry individual metric classes#16064trask merged 3 commits intoopen-telemetry:mainfrom
Conversation
39023c9 to
9a8a9bb
Compare
9a8a9bb to
b4d9d99
Compare
| observables.addAll(MemoryPools.registerObservers(openTelemetry)); | ||
| observables.addAll(Threads.registerObservers(openTelemetry)); | ||
| observables.addAll( | ||
| io.opentelemetry.instrumentation.runtimemetrics.java8.Classes.registerObservers( |
There was a problem hiding this comment.
Is there a good reason to use FQN here ?
There was a problem hiding this comment.
this is usually done so because importing deprecated classes causes warnings that can't be suppressed and the build is configured to fail on warnings
| observables.addAll( | ||
| io.opentelemetry.instrumentation.runtimemetrics.java8.Threads.registerObservers( | ||
| openTelemetry)); | ||
| if (emitExperimentalTelemetry) { |
There was a problem hiding this comment.
If we suggest using metric views to filter experimental metrics, does this means we would also get rid of this condition or do we still keep it ? From what I understand here the goal for now is only to make the individual metric classes internal.
There was a problem hiding this comment.
I think we keep it, at least for now. We could consider removing it after open-telemetry/opentelemetry-specification#4809, though we could also keep it even then as "emit experimental telemetry" is a pattern we use throughout.
| runtimeMetrics.close(); | ||
| ``` | ||
|
|
||
| To select specific metrics, configure [metric views](https://opentelemetry.io/docs/specs/otel/metrics/sdk/#view) |
There was a problem hiding this comment.
Do we have an example implementation with Java SDK we could link to ?
| * @deprecated Use {@link io.opentelemetry.instrumentation.runtimemetrics.java8.RuntimeMetrics} | ||
| * instead, and configure metric views to select specific metrics. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
since this is an internal class we don't really need to do anything with it
There was a problem hiding this comment.
I tend to think of .internal.Experimental* as one step above other .internal classes. since easy in this case (and doesn't require annoying copy), I'd suggest go ahead with deprecation (but agree we don't necessarily need to)
| * @deprecated Use {@link RuntimeMetrics} instead, and configure metric views to select specific | ||
| * metrics. | ||
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
if your goal is just to move these to internal then maybe it would be easier to move the class, drop final, add deprecated class in original location that extends the moved class
There was a problem hiding this comment.
Pull request overview
This pull request deprecates individual runtime telemetry metric classes in favor of using the RuntimeMetrics API with metric views to control which metrics are emitted. This change simplifies the public API by moving implementation classes to an internal package while maintaining backward compatibility through deprecated wrapper classes.
Changes:
- Moved implementation classes (Classes, Cpu, GarbageCollector, MemoryPools, Threads) from public API to internal package
- Deprecated public API classes with delegation to internal implementations
- Marked experimental metric classes (ExperimentalBufferPools, ExperimentalCpu, ExperimentalMemoryPools, ExperimentalFileDescriptor) as deprecated
- Updated README with comprehensive examples showing how to use RuntimeMetrics with metric views for selective metric collection
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
Classes.java (public API) |
Converted to deprecated wrapper that delegates to internal implementation |
Cpu.java (public API) |
Converted to deprecated wrapper that delegates to internal implementation |
GarbageCollector.java (public API) |
Converted to deprecated wrapper that delegates to internal implementation |
MemoryPools.java (public API) |
Converted to deprecated wrapper that delegates to internal implementation |
Threads.java (public API) |
Converted to deprecated wrapper that delegates to internal implementation |
Classes.java (internal) |
New internal implementation class moved from public package |
Cpu.java (internal) |
New internal implementation class moved from public package |
GarbageCollector.java (internal) |
New internal implementation class moved from public package |
MemoryPools.java (internal) |
New internal implementation class moved from public package |
Threads.java (internal) |
New internal implementation class moved from public package |
ExperimentalBufferPools.java |
Added @deprecated annotation with guidance to use RuntimeMetrics |
ExperimentalCpu.java |
Added @deprecated annotation with guidance to use RuntimeMetrics |
ExperimentalMemoryPools.java |
Added @deprecated annotation with guidance to use RuntimeMetrics |
ExperimentalFileDescriptor.java |
Added @deprecated annotation with guidance to use RuntimeMetrics |
JmxRuntimeMetricsFactory.java |
Updated to reference internal classes directly, added deprecation suppression |
ClassesTest.java |
Moved to internal package with updated imports |
CpuTest.java |
Moved to internal package with updated imports |
GarbageCollectorTest.java |
Moved to internal package with updated imports |
MemoryPoolsTest.java |
Moved to internal package with updated imports |
ThreadsTest.java |
Moved to internal package with updated imports |
ExperimentalBufferPoolsTest.java |
Added deprecation suppression annotation |
ExperimentalCpuTest.java |
Added deprecation suppression annotation |
ExperimentalMemoryPoolsTest.java |
Added deprecation suppression annotation |
ExperimentalFileDescriptorTest.java |
Added deprecation suppression annotation |
README.md |
Updated with new usage examples showing RuntimeMetrics API and metric view configuration |
Towards
I think metric view are good enough to include/exclude individual metrics as needed, so can remove these from public API.
cc @roberttoyonaga